Skip to content

Make opaque types decomposable #24068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 7, 2025
Merged

Make opaque types decomposable #24068

merged 2 commits into from
Oct 7, 2025

Conversation

nox213
Copy link
Contributor

@nox213 nox213 commented Sep 25, 2025

fixes: #22513

Currently, if the upper bounds of opaque types are sealed, they pass exhaustivityCheckable, but exhaustivity checks are not handled correctly because the compiler preventing from decomposing them.
The guard was introduced in #19368, but the tests still pass even without this guard.

@nox213
Copy link
Contributor Author

nox213 commented Sep 25, 2025 •
edited
Loading 

Errors found on incorrect row numbers when compiling tests/vulpix-tests/unit/negAnnotWrongLine.scala
-> following the errors:
 at 2: Found:    ("LOL" : String)
Required: Int
[                                        ] completed (0/1, 0 failed, 0s)
[=======================================>] completed (1/1, 1 failed, 0s)
[info] Test dotty.tools.vulpix.VulpixUnitTests.infiniteAlloc started
[info] Test dotty.tools.vulpix.VulpixUnitTests.runOutRedirects started
[info] Test dotty.tools.vulpix.VulpixUnitTests.infiteNonRec started
[info] Test dotty.tools.vulpix.VulpixUnitTests.negNoPositionAnnot started


Errors found on incorrect row numbers when compiling tests/vulpix-tests/unit/negNoPositionAnnots.scala
-> following the errors:
 at 2: Found:    ("LOL" : String)
Required: Int
[                                        ] completed (0/1, 0 failed, 0s)
[=======================================>] completed (1/1, 1 failed, 0s)
[info] Test dotty.tools.vulpix.VulpixUnitTests.runCompileFail started
[info] Test dotty.tools.vulpix.VulpixUnitTests.infiteTailRec started
[info] Test dotty.tools.vulpix.VulpixUnitTests.badJava started
[info] Test dotty.tools.vulpix.VulpixUnitTests.pos1Error started
[info] Test dotty.tools.vulpix.VulpixUnitTests.deadlock started
[info] Test dotty.tools.vulpix.VulpixUnitTests.negTooManyAnnots started


Wrong number of errors encountered when compiling tests/vulpix-tests/unit/negTooManyAnnots.scala
expected: 2, actual: 1
Unfulfilled expectations:
tests/vulpix-tests/unit/negTooManyAnnots.scala:2
Unexpected errors:

-> following the errors:
 at 2: Found:    ("LOL" : String)
Required: Int

It seems not related to my fix.

@bishabosha
Copy link
Member

bishabosha commented Sep 25, 2025 •
edited
Loading

It seems not related with my fix.

to confirm this, does it still fail if you revert your change? - there can always be knock-on effects

@nox213
Copy link
Contributor Author

nox213 commented Sep 25, 2025

It seems not related with my fix.

to confirm this, does it still fail if you revert your change? - there can always be knock-on effects

I see, I tried to run locally, but I couldn't because of

package scala.compiletime does not have a member method summonFrom
errors.

@som-snytt
Copy link
Contributor

Me too, but

sbt:scala3> scala3-bootstrapped-new/testOnly dotty.tools.vulpix.VulpixUnitTests

I'll take a look. It's also confusing when tests emit junk text. But this looks like an actual fail.

[=======================================>] completed (1/1, 1 failed, 3s)

Also I can't stand the progress bars at this point, but is there any point in complaining.

@nox213
Copy link
Contributor Author

nox213 commented Sep 25, 2025 •
edited
Loading

Somehow, test ran locally, and it was an actual fail. I will look into it.

@nox213
Copy link
Contributor Author

nox213 commented Sep 25, 2025 •
edited
Loading

Sorry for the confusion, I got same errors even though I tested on main branch(without my changes)
image

@nox213
Copy link
Contributor Author

nox213 commented Sep 25, 2025 •
edited
Loading

#24063
This PR failed at the same point.
https://github.com/scala/scala3/actions/runs/17987046738/job/51171294736

@som-snytt
Copy link
Contributor

The summary is correct; the "progress" noise is just noise. But it has an option to suppress it, which I will propose.

[info] Test run dotty.tools.vulpix.VulpixUnitTests finished: 0 failed, 0 ignored, 19 total, 23.642s
[info] Passed: Total 19, Failed 0, Errors 0, Passed 19

I can't read the linked PR output with the progress bar "supernoise", so I can't comment.

@som-snytt som-snytt mentioned this pull request Sep 25, 2025
Merged
zielinsky
zielinsky requested changes Oct 3, 2025
View reviewed changes
@nox213 nox213 requested a review from zielinsky October 3, 2025 11:39
zielinsky
zielinsky approved these changes Oct 7, 2025
View reviewed changes
Copy link
Member

@zielinsky zielinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zielinsky zielinsky merged commit f4e6239 into scala:main Oct 7, 2025
75 of 76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zielinsky zielinsky zielinsky approved these changes

@hamzaremmal hamzaremmal Awaiting requested review from hamzaremmal

Assignees

@zielinsky zielinsky

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect exhaustivity warning when using opaque types with constraint and enums

4 participants

@nox213 @bishabosha @som-snytt @zielinsky